build your first basic workflow experimentation #3994
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
jsundai
changed the title
📖 Docs PR preview links |
docs/build-your-first-basic-workflow/build-your-first-workflow.mdx
Outdated
Show resolved
Hide resolved
|
|
||
| </SdkTabs.Java> | ||
|
|
||
| <SdkTabs.TypeScript> |
There was a problem hiding this comment.
Minor thing, but could we have the text wrap button in here like we have on other code blocks for all of these on the page? Example
docs/build-your-first-basic-workflow/build-your-first-workflow.mdx
Outdated
Show resolved
Hide resolved
| import type { PaymentDetails } from './shared'; | ||
|
|
||
| // highlight-next-line | ||
| export async function moneyTransfer(details: PaymentDetails): Promise<string> { // Workflow Definition: an exported async function |
There was a problem hiding this comment.
Nit: could the // Workflow Definition: an exported async function go above // highlight-next-line to keep it consistent with all the other comments?
| let withdrawResult: string; | ||
| try { | ||
| // highlight-next-line | ||
| withdrawResult = await withdraw(details); // Executes Activity via proxy and waits for result |
There was a problem hiding this comment.
Nit: could // Executes Activity via proxy and waits for result go above // highlight-next-line to keep it consistent with all the other comments?
| </div> | ||
| }> | ||
|
|
||
| Now that your Worker is running and polling for tasks, you can start a Workflow Execution. |
There was a problem hiding this comment.
I might have missed it, but was there somewhere we told the users they would need 3 terminals? It confused me for a second that the first terminal we talked about under the text is Terminal 3.
| </SdkTabs> | ||
| </div> | ||
| <div style={{marginTop: '2rem', padding: '1rem', background: 'rgba(16, 185, 129, 0.1)', borderRadius: '0.5rem'}}> | ||
| <strong>Expected Success Output:</strong> |
There was a problem hiding this comment.
Could we put which terminal this should show in? It looks like Terminal 2.
|
|
||
| ## Check the Temporal Web UI | ||
|
|
||
| <SetupStep code={ |
There was a problem hiding this comment.
Nit: could this open in a new tab instead of the docs one?
| const bank = new BankingService('bank2.example.com'); | ||
|
|
||
| // Comment out this working line: | ||
| // return await bank.deposit(details.targetAccount, details.amount, details.referenceId); |
There was a problem hiding this comment.
It might just be a formatting thing, but this looks different than the repo and the comment/uncomment snippets are in a different order (the good code is on top of the bug code).
There was a problem hiding this comment.
Good catch! I'll review all the code snippets and make sure it matches the repo. I added in some comments but it might be better to remove them for consistency.
| Try this advanced scenario of compensating transactions. | ||
|
|
||
|
|
||
| 1. **Modify the retry policy** in `workflows.py` to only retry 1 time |
There was a problem hiding this comment.
Do we only want to highlight Python here or all the languages?
….mdx Co-authored-by: Milecia McG <47196133+flippedcoder@users.noreply.github.com>
….mdx Co-authored-by: Milecia McG <47196133+flippedcoder@users.noreply.github.com>
| cd money-transfer-project-template-python | ||
| </CodeSnippet> | ||
| <CodeSnippet language="bash"> | ||
| python -m pip install temporalio |
There was a problem hiding this comment.
Should we include the virtual env set up commands here?
python -m venv .venv
source .venv/bin/activate
python -m pip install temporalio
| **workflows.py** | ||
|
|
||
| ```python | ||
| from datetime import timedelta |
There was a problem hiding this comment.
I just noticed this code is hard-coded in the docs instead of using snipsync. Is that something we want to do or could the highlights be added directly to the code repo so everything stays consistent?
| workflow.logger.info( | ||
| f"Refund successful. Confirmation ID: {refund_output}" | ||
| ) | ||
| raise deposit_err |
There was a problem hiding this comment.
This code doesn't match what's in the repo: https://github.com/temporalio/money-transfer-project-template-python/blob/main/workflows.py#L53-L61
| from shared import PaymentDetails | ||
|
|
||
| class BankingActivities: | ||
| # highlight-next-line |
There was a problem hiding this comment.
This code should be included to show where self.bank comes from.
|
|
||
| ```python | ||
| # Task Queue name - used by both Worker and Workflow starter | ||
| MONEY_TRANSFER_TASK_QUEUE_NAME = "MONEY_TRANSFER_TASK_QUEUE" |
| **run_workflow.py** | ||
|
|
||
| ```python | ||
| import asyncio |
There was a problem hiding this comment.
I'm a little confused here. This code is different from the repo file. Could we use the snipsync here?
There was a problem hiding this comment.
The green text is a bit hard to see on light mode.
| ## Step 4: Define the Task Queue | ||
|
|
||
| A Task Queue is where Temporal Workers look for Tasks about Workflows and Activities to execute. | ||
| Each Task Queue is identified by a name, which you will specify when you configure the Worker and again in the code that starts the Workflow Execution. |
There was a problem hiding this comment.
Is this a good place to mention that mismatched task queues don't result in error but result in workers not getting any task? I think it's a good callout to make and it'd be short
|
|
||
| ### Step 3: Simulate the Crash | ||
|
|
||
| **The moment of truth!** Kill your Worker while it's processing the transaction. |
There was a problem hiding this comment.
Do I maybe not have the latest code for the workflow? Because right now if I follow this sequence, the transfer completes almost instantly and I don't really get to stop the worker while it's processing the transaction
|
|
||
| Before you begin, set up your local development environment: | ||
|
|
||
| <CallToAction |
There was a problem hiding this comment.
I know it's out of scope for this, but just wanted to point out that there isn't a PHP quick start. 😅
|
|
||
| This is what the Workflow Definition looks like for the money transfer process: | ||
|
|
||
| **MoneyTransfer.php** |
There was a problem hiding this comment.
It might help to have the full path here. I had to click around a little before I found the file. src/Workflow/MoneyTransfer.php
|
|
||
| Activities handle the business logic. Each Activity method calls an external banking service: | ||
|
|
||
| **BankingActivity.php** |
There was a problem hiding this comment.
It might help to have the full path here. I had to click around a little before I found the file. src/Banking/Internal/BankingActivity.php
| declare(strict_types=1); | ||
|
|
||
| namespace App\Banking\Internal; | ||
|
|
There was a problem hiding this comment.
Should probably include this import since it's referenced below. https://github.com/temporalio/money-transfer-project-template-php/blob/main/src/Banking/Internal/BankingActivity.php#L7
| $data->amount, | ||
| $referenceId, | ||
| ); | ||
| // In Part 2, uncomment the block below and comment out the line above |
There was a problem hiding this comment.
Just checking if you want this Part 2 comment here.
|
|
||
| <SdkTabs.PHP> | ||
|
|
||
| **Worker.php** |
There was a problem hiding this comment.
You might want to include the path here too, like with the others. src/worker.php
| **shared.php** or in your Worker.php | ||
|
|
||
| ```php | ||
| <?php |
There was a problem hiding this comment.
I didn't find a reference to this anywhere in the PHP repo. This probably needs to be added to it.
|
|
||
| <SdkTabs.PHP> | ||
|
|
||
| **transfer.php** |
There was a problem hiding this comment.
Should be src/transfer.php to match the others.
| cd money-transfer-project-template-php | ||
| </CodeSnippet> | ||
| <CodeSnippet language="bash"> | ||
| composer install |
There was a problem hiding this comment.
It looks like the money-transfer repo for this can't install the packages with just this command. 😅 I had to run composer update --ignore-platform-req=ext-grpc and then composer install --ignore-platform-req=ext-grpc to get the install to work. But when I tried to run php src/transfer.php, it actually needs grpc to make the calls. So I don't think a user could easily run this without digging into it.
|
.NET feedback: I think there may be some versioning issues in .NET. Although .NET 10 is the current version, and uses a slightly different program architecture, I needed to install .NET 8 to get the worker to run. |
|
|
||
| <SdkTabs.DotNet> | ||
|
|
||
| **Shared/Constants.cs** |
There was a problem hiding this comment.
Shared/Constants.cs isn't included in the sample repo. It's the only file in the .NET example that I had to create manually. We should probably update the example rather than making a change to the instructions.
| <CodeSnippet language="bash">npm run worker</CodeSnippet> | ||
| </SdkTabs.TypeScript> | ||
| <SdkTabs.DotNet> | ||
| <CodeSnippet language="bash">dotnet run --project MoneyTransferWorker</CodeSnippet> |
There was a problem hiding this comment.
As best I can tell, this needs to include the file extension to work.
dotnet run --project MoneyTransferWorker.csproj
| <CodeSnippet language="bash">npm run client</CodeSnippet> | ||
| </SdkTabs.TypeScript> | ||
| <SdkTabs.DotNet> | ||
| <CodeSnippet language="bash">dotnet run --project MoneyTransferClient</CodeSnippet> |
There was a problem hiding this comment.
I believe this needs the .csproj extension in order to work.
What does this PR do?
https://temporal-documentation-git-tutorial-layout-money-transfe-0fc0c4.preview.thundergun.io/build-your-first-basic-workflow/build-your-first-workflow
Notes to reviewers
files changed include new components for interaction and sdk boxed logos
wip (TODO: make light mode friendly)
aiming for: concise + engaging, dev-to-dev tone, consistent interactions, progressive disclosure.
next iteration: can include sdk box logos